Skip to content

Allow using hosted Redis instances by default#293

Closed
schneems wants to merge 5 commits into
Shopify:mainfrom
schneems:schneems/allow-hosted-redis
Closed

Allow using hosted Redis instances by default#293
schneems wants to merge 5 commits into
Shopify:mainfrom
schneems:schneems/allow-hosted-redis

Conversation

@schneems
Copy link
Copy Markdown
Contributor

Hosted Redis servers use self signed certificates (because they do not own the domain they're running on such as compute-1.amazonaws.com) therefore a full SSL connection verification will fail. The fix for that behavior is to disable verification so a self signed certificate can be used docs from a hosted Redis/Key-value store provider.

This PR makes the default behavior to allow connecting to self signed Redis servers, and introduces a new flag --strict-ssl which will require that the certificate is not self-signed.

This failing SSL connection behavior was originally reported in #292, however that issue focues on raising visibility of such failures.

@casperisfine casperisfine marked this pull request as ready for review November 13, 2024 07:37
@casperisfine
Copy link
Copy Markdown
Contributor

Sorry, but you'll have to sign the CLA...

As for the PR, I'm not even sure we need a flag. I can't really think of a security issue caused by CI queue being pointed at the wrong Redis, unless somehow the list of test name is super secret.

Copy link
Copy Markdown
Contributor

@ChrisBr ChrisBr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution

@schneems
Copy link
Copy Markdown
Contributor Author

I have signed the CLA!

@schneems
Copy link
Copy Markdown
Contributor Author

I'm not even sure we need a flag.

I don’t feel strongly about it. I generally find changes to defaults easier to get merged if there’s some sort of opt out so I did what I thought would be the path of least resistance. I have no strong reason to force full SSL checking.

It sounds like you prefer fewer config options over greater configurability. Seems reasonable.

Please confirm and I can rip that out (or feel free to take the otherwise quite simple change and run with it if it’s faster/easier for you).

schneems added a commit to schneems/ci-queue that referenced this pull request Nov 19, 2024
Hosted Redis servers use self signed certificates (because they do not own the domain they're running on such as `compute-1.amazonaws.com`) therefore a full SSL connection verification will fail. The fix for that behavior is to disable verification so a self signed certificate can be used [docs from a hosted Redis/Key-value store provider](https://devcenter.heroku.com/articles/connecting-heroku-redis#connecting-in-ruby).

This PR makes the default behavior to allow connecting to self signed Redis servers. 

This failing SSL connection behavior was originally reported in Shopify#292, however that issue focuses on raising visibility of such failures.

This is an alternative to Shopify#293 that does not introduce a new configuration flag.
@schneems
Copy link
Copy Markdown
Contributor Author

I also opened #294 which doesn't include the configuration flags.

@ChrisBr
Copy link
Copy Markdown
Contributor

ChrisBr commented Dec 3, 2024

Superseded by #294

@ChrisBr ChrisBr closed this Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants